Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make exercise by interface contract ID safe #14134

Merged
merged 32 commits into from
Aug 31, 2022

Conversation

S11001001
Copy link
Contributor

@S11001001 S11001001 commented Jun 8, 2022

Fixes #14132.

CHANGELOG_BEGIN
- [TypeScript codegen] To exercise an interface choice on a
  template-typed contract ID, you must now explicitly convert the
  contract ID to an interface-typed ID.  For example, where ``cid:
  ContractId<TemplateName>``, you can use
  ``TemplateName.toInterface(InterfaceName, cid)`` to produce a
  ``ContractId<InterfaceName>``.

  Interface contract IDs are also now typed as shown above; for example,
  the former type ``ContractId<InterfaceNameInterface<TemplateName>>``
  would now simply be ``ContractId<InterfaceName>``.
CHANGELOG_END
  • find a working approach
  • remove dev stubs
  • changelog

@S11001001 S11001001 added component/js-ecosystem TypeScript and React.js bindings team/ledger-clients Related to the Ledger Clients team's components. labels Jun 8, 2022
@S11001001 S11001001 self-assigned this Jun 8, 2022
@cocreature
Copy link
Contributor

Maybe for some useful context: The way I’m imaging most of our usage will look like is that we’ll call an interface choice on an interface contract id. I don’t actually care much about being able to call an interface choice on a template contract id. So I think a design where we have:

  1. Typechecked conversions from a template contract id to an interface contract id provided there is an implementation.
  2. Typechecked exercises of interface choices on interface contract ids.
  3. No exercise of interface choices on template contract ids (maybe with some fallback).

is worth considering imho.

@S11001001
Copy link
Contributor Author

I have a prototype interface that appears to do the right things, accepting and rejecting the proper expressions, as explained in the sample function therein.

Specifically, this makes quite a different representation of interface contract IDs from that in #11280 (and will remove mbSubst); @Robin-da do you foresee any problems regarding this in the sample linked above?

Copy link
Contributor

@cocreature cocreature left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I like it. The "hack" to avoid the variance issues in ToInterface doesn’t feel particularly nice but I amit I don’t have a better idea. What happens if we drop that? Does that make all casts typecheck or only if you specify never explicitly as the type?

I’d also be interested to see examples of:
1 Interplay with retroactive implementations. In your example you seem to extend TI from T. It’s not clear to me how this is extended to reotractive implementations where you don’t know the set of implemented interfaces upfront.
2. Interplay with interface hierarchies

@S11001001
Copy link
Contributor Author

What happens if we drop that? Does that make all casts typecheck or only if you specify never explicitly as the type?

I'm not really sure. The problem with predicting this is that, as the comment mentions, a surprising number of type expressions are = to never.

All of the examples in the file work out the same if I drop the conditional. But I can then pass I3 & I2 as a type parameter to toInterface to (invalidly) convert a T contract ID to an I3 contract ID. (I3 & I2 = never.) I can also pass I1 & I2 to get an I3 contract ID, for the same reason. Are there cases where these would be inferred? Are there other surprising, perhaps more inferrable types that would = never and thus perform this coercion? I don't know.

where you don’t know the set of implemented interfaces upfront.

This will never happen, even with retroactives. We codegen on the whole environment.

Interplay with interface hierarchies

None of the codegens model these at all, only template/interface relationships.

@cocreature
Copy link
Contributor

This will never happen, even with retroactives. We codegen on the whole environment.

This is not really how daml2js works atm. Yes it has a complete environment in theory but the way it process things atm it looks at one package at a time and its dependencies not the whole package. I also don’t think it’s as simple as just looking at the whole environment:

Consider this example:

  1. Package A defines template T
  2. Package B defines interface I and a retroactive instance for T

Daml2js generates each LF package as a separate JS package. In this example you would thus have two JS package.
Package B can definitely depend on package A (after all it does depend on it in Daml).
With your approach you introduce a dependency from package A on package B.
So now you have a circular dependency and things go bad.

@S11001001
Copy link
Contributor Author

Yes it has a complete environment in theory but the way it process things atm it looks at one package at a time and its dependencies not the whole package.

We can deal with this, since we can pass extra arguments about the environment. #14082

Package B can definitely depend on package A (after all it does depend on it in Daml).

I agree this is a problem. The module dependencies end up potentially circular and breaking here.

I'd rather not add toInterface to the interfaces and have users have to know whether it was retroactive or not to know which method to call. Perhaps a function in @daml/types toInterface(TC, IC, cid) that has type overloads to either use type information in the Template Companion or the Interface Companion? (Not having typeclasses is really unfortunate.)

@cocreature
Copy link
Contributor

toInterface(TC, IC, cid) that has type overloads to either use type information in the Template Companion or the Interface Companion? (Not having typeclasses is really unfortunate.)

That sounds sensible to me. I think it’s worth thinking about whether we can use a similar design in the JVM codegens but that’s a separate conversation.

@S11001001
Copy link
Contributor Author

@chunlokling-da and I reworked the prototype to properly support retroactive implementations; see the new version here. Key changes:

  1. Extra argument to toInterface and unsafeFromInterface, goal is to have the API be consistent for TypeScript users no matter whether the implementation is retroactive (RI4) or not
  2. Conditional removed; the fact that you must pass in companion evidence makes it no longer necessary

What do you think @cocreature ?

CHANGELOG_BEGIN
- [TypeScript codegen] To exercise an interface choice on a
  template-typed contract ID, you must now explicitly convert the
  contract ID to an interface-typed ID.  For example, where ``cid:
  ContractId<TemplateName>``, you can use
  ``TemplateName.toInterface(InterfaceName, cid)`` to produce a
  ``ContractId<InterfaceName>``.

  Interface contract IDs are also now typed as shown above; for example,
  the former type ``ContractId<InterfaceNameInterface<TemplateName>>``
  would now simply be ``ContractId<InterfaceName>``.
CHANGELOG_END
@S11001001 S11001001 added the roadmap/interfaces https://digitalasset.atlassian.net/browse/DAML-56 label Aug 30, 2022
@S11001001 S11001001 marked this pull request as ready for review August 30, 2022 21:05
@S11001001 S11001001 requested a review from a team August 30, 2022 21:06
@S11001001 S11001001 merged commit 85f93f5 into main Aug 31, 2022
@S11001001 S11001001 deleted the 14132-exercise-interface-contract-id-safe branch August 31, 2022 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/js-ecosystem TypeScript and React.js bindings roadmap/interfaces https://digitalasset.atlassian.net/browse/DAML-56 team/ledger-clients Related to the Ledger Clients team's components.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

constrain contract ID types when invoking interface-member choices
3 participants